Skip to content

draft: UTXO reservation wallet-side foundations - #4238

Draft
mswilkison wants to merge 22 commits into
reservations-epicfrom
feat/utxo-reservation-wallet-support
Draft

draft: UTXO reservation wallet-side foundations#4238
mswilkison wants to merge 22 commits into
reservations-epicfrom
feat/utxo-reservation-wallet-support

Conversation

@mswilkison

@mswilkison mswilkison commented Aug 7, 2026

Copy link
Copy Markdown
Contributor

Companion of threshold-network/tbtc-v2#1088 (UTXO reservations: segregated custody with in-kind redemption). A reservation is a deposit the wallet anchors — a 1-input-1-output spend into a fresh wallet-controlled output with no refund path — instead of sweeping, so reserved coins never commingle with the pooled supply and are redeemable in-kind by their owner.

What's included

  • Wallet action types for the four lifecycle actions (anchor, reserved redemption, re-anchor, dissolution), appended after the existing enum values to preserve serialized compatibility.
  • Coordination proposal types implementing CoordinationProposal, registered in the unmarshaling factory. Marshaling is JSON-based for now with an explicit TODO — switching to protobuf requires adding the reservation message types to the coordination proto definition and regenerating pkg/tbtc/gen/pb.
  • Chain interface extensions: GetReservation, GetReservationAction, GetReservationParameters, GetReservationTotalAmount, and the four ValidateReservation*Proposal methods mapping onto the new WalletProposalValidator views from the contracts PR, plus ComputeReservationRedeemerOutputScriptHash — implemented (not stubbed), since it needs no new ABI, only the existing keccak256 rule already used by buildRedemptionKey. Note: the nonce-keyed GetReservationAction(reservationKey, requestNonce) lookup and the terminal ReservationActionState values (Settled/TimedOut/Vetoed/Superseded) model the anticipated two-phase authorize-then-prove settlement redesign tracked in tbtc-v2#1088's own review findings, not the currently-reviewed single-phase (request→prove) contract — this interface may change once the contracts PR's final shape lands.
  • Unsigned transaction assembly for all four lifecycle shapes, enforcing the 1-in-1-out lineage rules the Bridge proves (dissolution additionally spends the wallet main UTXO as its second input; the anchor outpoint is placed first, one of the two input orders the Bridge's dissolution proof accepts by outpoint-hash match rather than position).
  • Tests: action parsing, proposal marshaling roundtrips, assembler input validation. go test ./pkg/tbtc/ passes in full.

Deliberately deferred (and why)

  1. Ethereum bindings: TbtcChain stubs the new methods with descriptive errors. The generated contract bindings can only be regenerated once the reservation Bridge ABI is published with the @keep-network/tbtc-v2 package — i.e., after the contracts PR merges.
  2. Coordination executor wiring + tbtcpg proposal generation: both consume the bindings above, so they land in the same follow-up. The assembly and validation layers they will call are what this PR provides.
  3. Protobuf marshaling for the proposal types (see TODO markers).
  4. SPV maintainer proof path (proof type registration + submitter): the design's credit mechanism hinges on SPV proofs of the anchor/redemption/re-anchor transactions, but wiring a new proof type into the SPV maintainer depends on the same unpublished reservation Bridge ABI as the Ethereum bindings above, so it lands with them.

Note for maintainers

origin/main currently fails to build (pkg/tbtcpg/redemptions.go:225: the Chain interface was refactored to return tbtc.RedemptionParameters as a struct, but the fee-estimation call site still destructured the old 8-value tuple) — the Client workflow is red on the main tip as well. This PR carries the one-line repair as a separate labeled commit so CI can run green here; feel free to cherry-pick it to main independently of the reservation work.

Companion of the tbtc-v2 UTXO reservation draft (threshold-network/
tbtc-v2#1088). A reservation is a deposit the wallet anchors -- spends
in a 1-input-1-output transaction into a fresh wallet-controlled output
with no refund path -- instead of sweeping, so the reserved coins never
commingle with the pooled supply and are redeemable in-kind.

Adds the wallet-side foundations:
- wallet action types for the four reservation lifecycle actions
  (anchor, reserved redemption, re-anchor, dissolution), appended after
  the existing enum values to preserve serialized compatibility,
- coordination proposal types with marshaling and factory registration
  (JSON-based for now; switching to protobuf once the reservation
  message types are added to the coordination proto definition),
- Chain interface extensions for reading reservations and parameters
  and validating the four proposal kinds via WalletProposalValidator,
- unsigned transaction assembly for all four lifecycle shapes,
  enforcing the 1-input-1-output lineage (dissolution additionally
  spends the wallet main UTXO as its second input, per the Bridge
  rules),
- tests for action parsing, proposal marshaling roundtrips, and
  assembler input validation.

The Ethereum chain implementation stubs the new interface methods with
descriptive errors: the contract bindings can only be regenerated once
the reservation Bridge API is published with the @keep-network/tbtc-v2
package. Coordination executor wiring and tbtcpg proposal generation
follow in the same step.
@coderabbitai

coderabbitai Bot commented Aug 7, 2026

Copy link
Copy Markdown

Important

Draft PR not reviewed

Draft PRs are not automatically reviewed by default.

  • Trigger a manual review

To automatically review draft PRs, update your CodeRabbit configuration:

reviews:
  auto_review:
    drafts: true

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

Repairs a pre-existing build break on main: the tbtcpg Chain interface
was refactored to return tbtc.RedemptionParameters as a struct, but the
fee-estimation call site in redemptions.go still destructured the old
8-value tuple. All other call sites already use the struct form.
@piotr-roslaniec
piotr-roslaniec changed the base branch from main to reservations-epic August 19, 2026 16:53
…through chain

Addresses confirmed findings from a multi-agent review of the reservation
wallet-side foundations:

- assembleReservationAnchorTransaction/assembleReservationReanchorTransaction
  now take an action snapshot and enforce the action's TxMaxFee ceiling and
  the reservation minimum amount floor, mirroring the redemption/dissolution
  assemblers.
- assembleReservedRedemptionTransaction and assembleReservationReanchorTransaction
  enforce the reservation minimum amount floor on their remainder/re-anchor
  outputs.
- computeReservationRedeemerOutputScriptHash moved off pkg/tbtc (a
  host-chain-agnostic package) onto BridgeChain.ComputeReservationRedeemerOutputScriptHash,
  matching how ComputeMainUtxoHash is already delegated through the chain
  abstraction; the Ethereum implementation shares its keccak step with
  buildRedemptionKey.
- assembleReservationDissolutionTransaction rejects a wallet main UTXO that
  wasn't part of the action's snapshot instead of silently discarding it,
  and requires a bridge chain unconditionally; its bridgeChain parameter is
  now a narrow inline interface instead of the full BridgeChain.
- ReservationReanchorProposal.Unmarshal rejects a zero target wallet public
  key hash; ReservationAnchorProposal.Unmarshal rejects a zero deposit
  funding tx hash; all four proposal Unmarshal methods reject a
  non-positive or out-of-int64-range fee.
- ReservedRedemptionProposal carries its redeemer output script on the wire
  (previously unconstructible - no source supplied it).
- GetReservation/GetReservationAction/ReservationParameters moved from
  WalletProposalValidatorChain to BridgeChain, matching their Bridge-state-read
  peers.
- The 7 Ethereum reservation stubs return a wrapped sentinel error so
  callers can errors.Is() them; the mismatched dissolution localChain stub
  parameter name now matches its siblings.
- Marshal/Unmarshal for the four reservation proposals moved to
  marshaling.go alongside the other proposal marshalers; the reservation
  validity-block constants and the action type/state enums gained rationale
  and per-value doc comments.

Test coverage: happy-path and fee/value boundary tests for the anchor and
re-anchor assemblers (previously untested beyond a nil-input guard),
dissolution's action-amount/target-wallet mismatch checks, the
fee-exceeds-redemption-amount and partial-amount-exceeds-anchor-value
guards, and fuzz coverage for all four proposals' Unmarshal methods.
MetricName() and clientinfo.GetAllWalletActionTypes() are two
hand-maintained lists that must stay in sync; this pins that invariant so
drift fails the test suite instead of silently degrading metrics.
agent-docs/ holds review scratch output and should never be committed.
…ouble

LocalChain (pkg/tbtcpg's Chain test double) was missing the reservation
methods added to the BridgeChain/WalletProposalValidatorChain interfaces,
breaking staticcheck's compile of pkg/tbtcpg's tests. Adds panic-stub
implementations matching this file's existing convention for chain
functionality its tests don't exercise (e.g. ComputeMainUtxoHash).
assembleReservationAnchorTransaction was missing the action.ActionType ==
Acceptance and action.State == Pending guards that its redemption/
dissolution siblings both have (and that F1's original fix called for by
name). A stale or wrong-type action snapshot previously passed straight
through to fee/value validation instead of being rejected up front.
…nding

- delete the undeclared partial-redemption capability from
  assembleReservedRedemptionTransaction, restoring the strict
  1-input-1-output shape this PR's own body and the companion
  contracts spec both claim
- add the missing TargetWalletPublicKeyHash check to
  assembleReservationAnchorTransaction, matching the guard already
  enforced by its re-anchor and dissolution siblings
- move ReservationParameters to parameters.go alongside its
  siblings, drop the field-name stutter, and drop
  ReservationTotalAmount in favor of a dedicated accessor
- extract requireReservationAction/requireValidActionFee to remove
  the duplicated nil/type/state/fee guard blocks across all four
  assemblers
- narrow assembleReservedRedemptionTransaction's bridgeChain
  parameter to the single method it calls, matching the dissolution
  assembler's existing convention
- drop the interface-doc restatements on the four new proposal
  types' ActionType/ValidityBlocks methods
- add missing boundary test coverage (zero/negative fee, zero
  anchor value, zero redemption amount, nil action) and a fundUtxo
  test helper to de-duplicate funding-transaction setup
- change GetReservation to the (*Reservation, bool, error) found-flag
  convention already used by GetPendingRedemptionRequest, resolving
  the doc contradiction with GetReservationAction's error-on-not-found
  convention
- rename ReservationParameters() to GetReservationParameters(),
  returning by value like every sibling parameter getter, and add a
  dedicated GetReservationTotalAmount() accessor
- rename the shared redemption-key hashing helper from the
  reservation-only reservationRedeemerOutputScriptHash to
  redeemerOutputScriptHash since it names a rule shared by both
  buildRedemptionKey and the new Compute method, and restore the
  length-prefix doc comment dropped during extraction
- replace the %w-wrapped sentinel returns on the reservation stubs
  with direct returns, matching this file's existing stub convention
- add test coverage for the seven reservation stub methods and the
  renamed redeemer-output-script-hash helper
- clarify the source/target wallet split in
  ValidateReservationReanchorProposal's doc comment
- restore the blank line before ComputeReservationRedeemerOutputScriptHash's
  doc comment, matching the interface's blank-line-between-methods convention
- remove the dead, verbatim-duplicate ReservationKey nil check in
  ReservationDissolutionProposal.Unmarshal
- extract validateProposalNonceAndFee to remove the duplicated
  request-nonce/fee validation sequence across all four proposal
  Unmarshal methods
- add deterministic malformed-input tests (truncated JSON, negative
  fee, zero nonce) for each of the four new proposal types, since
  the existing fuzz targets only seed one well-formed input and
  exercise nothing under plain go test
…t bound

- drop the undisclosed agent-docs/ .gitignore addition, out of scope
  for this PR
- derive TestWalletActionType_MetricNameConsistency's loop bound from
  ParseWalletActionType's own domain instead of a hardcoded upper
  bound, so a future action type added without a clientinfo entry
  can't silently pass
- reanchor: bind action.Amount and the anchor outpoint, closing the
  only lane with no value/outpoint authorization (P1)
- anchor happy-path test: assert constructed inputs/outputs instead
  of just the returned error (P1)
- anchor/dissolution: derive the destination script from the wallet's
  own public key instead of trusting an RPC-supplied hash, matching
  every other self-paying assembler in the package
- redemption/dissolution/reanchor: thread the reservation's
  authoritative anchor outpoint through and reject on mismatch,
  instead of relying on a value-only check
- enforce ReservationAction.TimeoutAt in requireReservationAction
- reject an all-zero wallet/target public key hash in the anchor,
  reanchor, and dissolution assemblers to prevent a silent burn output
- restore validateMemberIndex's uint32->uint8 overflow guard
  (marshaling.go), dropped as unrelated collateral in this PR; keep
  the new zero-check alongside it
- range-check ReservationKey (sign, bit length) on the three
  JSON-unmarshaled reservation proposals
- simplify a redundant nil||len() check in
  ReservedRedemptionProposal.Unmarshal
- complete six truncated/missing doc comments on the new Ethereum
  chain reservation stubs; fix a misnamed and a misplaced comment
- dissolution: use builder.TotalInputsValue() instead of a hand-rolled
  sum; document the wallet-action enum as append-only
- add missing structural test coverage for reanchor/redemption
  inputs and five previously-untested fee/value boundary branches
- note the dissolution input-order assumption against the
  unmerged tbtc-v2#1088 Bridge contract as a tracked TODO
… comments

Silently deleted during the doc-comment fix for the reservation
Ethereum chain stubs, breaking TbtcChain's tbtcpg.Chain interface
implementation for FindDeposits/EstimateDepositsSweepFee/
NewProposalGenerator call sites in cmd/. Caught by CI (client-vet,
client-scan), not by 'go build ./pkg/...' alone since cmd/ isn't
under pkg/. Restored verbatim from the pre-fix commit.
EnsureWalletSyncedBetweenChains treated any 1-input-1-output
transaction spending a revealed deposit as an unproven first deposit
sweep and hard-errored. A reservation anchor transaction has the exact
same shape but deliberately never becomes the wallet's main UTXO,
permanently deadlocking wallet sync for any wallet holding a
reservation anchor.

Distinguish an anchor from a genuine unproven sweep by checking
whether the spent deposit's vault matches the reservation vault
(mirrors the existing sweep-vs-reservation check used elsewhere).
Treat GetReservationParameters failing (not yet implemented on every
chain backend) or an unset vault as "not a reservation" rather than
propagating the error, so ordinary deposit sweeps are unaffected.
…rage

- requireReservationAction now rejects a zero TimeoutAt as malformed
  instead of silently bypassing the timeout guard; adds test coverage
  for both the malformed-timeout and timed-out-action branches, which
  previously had none.
- Adds missing test coverage for the anchor-outpoint-mismatch guard in
  the redemption and dissolution assemblers (only re-anchor was
  tested), the nil-outpoint guards in all three UTXO-consuming
  assemblers, the wallet-public-key nil guard in the anchor and
  dissolution assemblers, the reachable target-wallet-hash guard in
  the re-anchor assembler, and the 1-input (no main UTXO) dissolution
  success path.
- Moves the unreachable TargetWalletPublicKeyHash zero-check in the
  anchor and dissolution assemblers ahead of the equality check it was
  shadowed by, so it provides real defense-in-depth.
- Replaces the dissolution assembler's stale TODO and inline comment
  asserting an unverified anchor-first input-order requirement: the
  companion Bridge contract (threshold-network/tbtc-v2#1088) accepts
  either input order by matching outpoint hashes, not position.
- Documents that the nonce-keyed GetReservationAction lookup and the
  terminal ReservationActionState values model the anticipated
  two-phase authorize-then-prove settlement redesign, not the
  currently-reviewed single-phase contract.
- Removes a redundant action-type parse duplicate of
  TestParseWalletActionType and decorative scenario comments that
  only restated the assertion below them.
The four reservation proposal Unmarshal implementations decoded
DepositFundingTxHash and TargetWalletPublicKeyHash into fixed-size
byte arrays directly, so a wrong-length JSON array silently zero-fills
or truncates instead of erroring (unlike the existing protobuf
unmarshalers, which reject a bad length explicitly). Unmarshal into an
intermediate []byte field first and reject a present-but-wrong-length
value before copying into the fixed array; an absent field still falls
through to the existing zero-value "required" check unchanged.

Also closes the equivalent oversized-fee gap already covered for
ReservationKey, and extends each proposal's protobuf-migration TODO
with the sequencing constraint: it must land before any code that
generates these proposals on the wire.
Documents the 0-means-disabled convention on the launch-throttle
fields (MinAmount, MaxTotalAmount, MaxReservationsPerWallet), and
replaces the repeated 'pending unpublished Bridge API' narrative on
the eight Ethereum reservation stub methods with a concise statement
of their current sentinel-error behavior.
piotr-roslaniec added a commit that referenced this pull request Sep 3, 2026
## Summary

Extends PR #4238 (proposal structs, marshaling, chain-interface stubs —
no executor) with the missing pieces: real ABI bindings for the
`ReservationRouter`, chain-interface implementations against those
bindings, the acceptance and re-anchor executors (proposal generation +
SPV proof submission), three monitoring watchers (stranding,
stale-deposit, action-timeout), and operator wiring gated behind
`config.Reservations.Enabled`.

**Repo:** threshold-network/keep-core
**Branch:** m1/keep-core-client
**Base:** reservations-epic
**Diffstat (this PR vs reservations-epic, working tree):** 84 files
changed, 25818 insertions(+), 72 deletions(-)

## Build pipeline (serial gates, parallel middle)

Ten commits, four sequential stages — later stages depend on earlier
ones' output, so this was not built as one flat diff:

1. `277865cb4` — regenerate Go ABI bindings for the reservation router
surface (`abi/gen`, `cmd/gen`, `contract/gen` — `ReservationRouter.go`
files, ~9,800 lines total across the three layers; plus incremental
diffs to
`Bridge.go`/`RedemptionWatchtower.go`/$WalletProposalValidator.go` for
the reservation-adjacent methods those contracts already exposed).
2. `e49e954e8` — reservation read/validate methods against the real
bindings, on `pkg/tbtc.Chain`.
3. `4ba2f2267` (`CurrentMonkey`) — reservation write methods, remaining
views, event subscriptions on `pkg/tbtc.Chain`.
4. `053577925` (gate 2.5, manager-run — narrow mechanical scope, not
delegated) — extends the same reservation methods onto
`pkg/tbtcpg.Chain` and `pkg/maintainer/spv.Chain`. This gate caught and
fixed a real defect first — see below.
5. Three genuinely independent builders, each its own worktree/branch
off `053577925`, then merged sequentially:
- `e37211b33` (`AcceptanceBuilder`) —
`pkg/tbtcpg/reservation_acceptance.go` (candidate selection + proposal
assembly) + `pkg/maintainer/spv/reservation_acceptance_proof.go` (SPV
proof submission).
   - `603ad0d54` (`ReanchorBuilder`) — the re-anchor equivalents.
- `ac58650a1` (`WatchersBuilder`) —
`pkg/maintainer/spv/reservation_{stranding,stale_deposit,action_timeout}_watch.go`.
- Merges: `2bd8731ce` (acceptance, clean), `daeb6afaf` (re-anchor,
clean), `c29de0b8f` (watchers — one real conflict, see below).
6. `48985451d` (`HilariousTermite`, serial operator wiring) — registers
both proposal tasks in `pkg/tbtcpg/tbtcpg.go`, both proof tasks in
`pkg/maintainer/spv/spv.go`, and the three watcher event subscriptions
in `pkg/tbtc/tbtc.go`, all gated behind `config.Reservations.Enabled`.
7. `b14a38851` — merge: pull in reservations-epic base updates
(bitcoin.Chain context-aware confirmation lookups) to fix local build
breakage.

## What ships (current state)

| Area | Files | Content |
|---|---|---|
| ABI bindings (generated) |
pkg/chain/ethereum/tbtc/gen/{abi,cmd,contract}/ReservationRouter.go +
incremental diffs to Bridge.go, RedemptionWatchtower.go,
WalletProposalValidator.go, LightRelay*.go | ~9,800 new lines, generated
from the ABI, not hand-written |
| Chain interface | pkg/chain/ethereum/tbtc.go (+1479),
pkg/tbtc/chain.go (+495/-…), pkg/tbtcpg/chain.go (+124),
pkg/maintainer/spv/chain.go (+111) | Read/write/event methods against
the real router surface. All reservation reads/writes/event
subscriptions target the **Bridge** address via fallback delegatecall,
never the router's own deployed address (see PR G's invariant note) |
| Coordination dispatch | pkg/tbtc/node_coordination.go (+18),
pkg/tbtc/node_proposals.go (+128), pkg/tbtc/coordination.go (+27) |
Wires agreed ActionReservationAnchor/ActionReservationReanchor proposals
to handleReservationAnchorProposal/handleReservationReanchorProposal,
which assemble, sign, and broadcast the anchor/re-anchor Bitcoin
transaction; checklist gating uses a chain-derived activation block, not
local config alone |
| Wallet action assembly | pkg/tbtc/reservation.go (+1032) |
assembleReservationAnchorTransaction/assembleReservationReanchorTransaction
and the wallet action types the dispatch above invokes |
| Acceptance executor | pkg/tbtcpg/reservation_acceptance.go (+714) |
ReservationAcceptanceTask — candidate selection, eligibility checks,
proposal assembly |
| Re-anchor executor | pkg/tbtcpg/reservation_reanchor.go (+472) |
Re-anchor proposal generation |
| SPV proof submission |
pkg/maintainer/spv/reservation_acceptance_proof.go (+109),
reservation_reanchor_proof.go (+312), reservation_proof_loop.go (+505) |
Builds and submits the SPV proof for each proposal type; the proof loop
is the driver that scans wallet transaction history and dispatches to
both submit functions |
| Watchers | pkg/maintainer/spv/reservation_stranding_watch.go (+106),
reservation_stale_deposit_watch.go (+299),
reservation_action_timeout_watch.go (+422) | Permissionless monitoring —
notify stranded/stale/timed-out reservations |
| Operator wiring | pkg/tbtcpg/tbtcpg.go (+24/-…),
pkg/maintainer/spv/spv.go (+12),
pkg/maintainer/spv/reservation_wiring.go (+417), pkg/tbtc/tbtc.go
(+42/-…), pkg/maintainer/spv/config.go (+23), cmd/start.go (+29/-…) |
Task/proof registration and event-subscription wiring, all behind
config.Reservations.Enabled |
| Metrics | pkg/clientinfo/performance.go (+61) | Reservation
action-type metric names, gated so a non-reservation deployment's
registered metric surface is unchanged |
| Tests | pkg/tbtc/{reservation,coordination,chain}_test.go,
pkg/tbtcpg/{reservation_acceptance,reservation_reanchor,chain,tbtcpg,fee,bitcoin_chain}_test.go,
pkg/maintainer/spv/{chain,reservation_acceptance_proof,reservation_proof_loop,reservation_wiring,reservation_action_timeout_watch,reservation_reanchor_proof,reservation_stale_deposit_watch,reservation_stranding_watch}_test.go,
pkg/chain/ethereum/tbtc_test.go,
pkg/tbtcpg/internal/test/{marshaling,reservation_acceptance,tbtcpgtest}.go,
16 JSON test-scenario fixtures, config/config_test.go +
test/config.{json,toml,yaml} | Unit coverage for every new production
file above, including the pure helpers in the SPV proof loop and wiring
layer |

## Two real bugs found and fixed mid-build

1. **Pointer-identity map-key bug** (`AcceptanceBuilder`’s output): the
test-double `reservationAcceptanceLocalChain.reservedDeposits` field was
typed `map[*big.Int]bool`. Go compares `*big.Int` map keys by pointer
identity, not value, so `IsReservedDeposit` always missed even deposits
the test had explicitly marked reserved, because `BuildDepositKey`
allocates a fresh pointer on every call. Fixed by re-keying on
`depositKey.Text(16)` (string).
2. **Zero-output funding-transaction stub**: the happy-path and
bounded-lookback test fixtures stubbed the funding Bitcoin transaction
as `&bitcoin.Transaction{}` (zero outputs), but
`ReservationAcceptanceTask.Run` genuinely calls
`assembleReservationAnchorTransaction`, which reads the funding output's
locking script to validate P2SH/P2WSH — so it panicked with "output
index out of range", then "not P2SH/P2WSH" once the first fix landed.
Fixed by giving the stub transactions a real single P2WSH output
(`0x0020` + 32 zero bytes) and adding the missing `ReservationTxMaxFee`
to the bounded-lookback scenario's `ReservationParameters` (was
defaulting to `0`, so any nonzero anchor fee tripped the "exceeds
configured max" guard).

## Defects caught during the build (both fixed, neither shipped)

- **Gate 2.5 subagent transiently malformed pkg/tbtc/chain.go** while
probing interface satisfaction (duplicate `ReservationChain` block,
methods declared after the interface's closing brace) and deleted the
tracked root main.go, then yielded a PARTIAL_COMPLETION reporting a
bogus SPV build failure it could not explain. Root-caused by the
manager: pkg/tbtc/chain.go was already clean at yield time; the main.go
deletion was the only lasting damage, restored via `git restore
--source=HEAD -- main.go`. The two interface extensions the subagent
actually delivered (pkg/tbtcpg/chain.go +137,
pkg/maintainer/spv/chain.go +97) were structurally correct; the
test-double mocks were missing three Past*Events stub methods, added by
hand. Committed clean…
- **Watchers merge conflict** (c29de0b, in
pkg/maintainer/spv/chain_test.go): two independently-built reservation
test-double representations — [16]byte/[24]byte-keyed maps from the
acceptance/re-anchor lineage vs map[string]-keyed maps re-added by the
watchers branch — plus a duplicate setReservation/setReservationAction
method pair and a dropped submitReservationProofHook field, all caught
and restored during resolution.
- **Build-brief self-correction before dispatch**: the initial build
brief recommended binding a separate read-only "router" handle to the
ReservationRouter's own deployed address for views/events. Verified
wrong against ReservationRouter.sol:71-76 invariant 3 ("NO STANDALONE
AUTHORITY") before any downstream subagent used it — the router's own
address has empty storage, so direct reads return garbage and every
event it emits carries the Bridge's address in the log, not the
router's. Fixed in the persisted brief before the bindings subagent
committed; a localChain unit-test mock would not have caught this since
it doesn't model per-address log-emitter semantics.

## Verification

- ✅ go build ./... — clean
- ✅ go test ./pkg/tbtc/... ./pkg/tbtcpg/... ./pkg/maintainer/spv/...
./pkg/clientinfo/... ./pkg/chain/ethereum/... -v — 340 passed, 0 failed
- ✅ go test ./... — full repo suite, 0 failed
- ✅ go vet ./... — clean except one pre-existing, unrelated issue at
pkg/tecdsa/signing/protocol.go:737 (not touched by this PR)
- ✅ gofmt -l (all changed files) — empty (no formatting issues)

## Review notes

- All reservation reads/writes/event subscriptions target the Bridge
address, never the router's own deployed address — this is a hard
invariant (ReservationRouter.sol:71-76), not a style choice. A reviewer
checking the chain-interface implementation should confirm every
reservation call site uses the bridge handle.
- Config-gated: every new task/proof/watcher registration is behind
config.Reservations.Enabled; with it unset (the default), this PR
changes no runtime behavior for existing (non-reservation) flows.
- The coordination checklist gate additionally requires a chain-derived
ReservationsActivationBlock alongside the local config flag, so a
mixed-rollout signing group (one operator flag-on, another flag-off)
can't cause a flag-off follower to fault an honest flag-on leader.
- No behavioral changes to non-reservation code paths — the diff is
additive except for the Chain interface files, coordination dispatch,
and operator-wiring files, which only add reservation-specific branches.
- Known scope limits, called out as an open follow-up rather than
blocking this PR:
- The action-timeout watcher's wallet-members resolver
(pkg/maintainer/spv/reservation_wiring.go) is a stub that always errors
"wallet members resolver not wired," so CheckReservationActionTimeouts
never reaches its notification path yet; the stranding and stale-deposit
watchers are fully live. Tracked to implement against GetOperatorID once
the on-chain accessor path is confirmed, mirroring
pkg/tbtc/inactivity.go's operator walk.
- ReservedRedemptionProposal/ReservationDissolutionProposal
marshal/assembly scaffolding in pkg/tbtc/reservation.go ships but is
inert for m1 (validator stubs explicitly error "not exposed on the m1
bridge-integration surface"); m1 only activates Acceptance/Reanchor.
Kept in this PR rather than split out since it shares the same
wire-format contract and is fully tested as shipped.
- ReservationAcceptanceTask keeps its stateful incremental-scan cache
(scanState, lastScannedBlock, pendingCandidates) rather than the
review-suggested redesign to a stateless full-rescan matching
DepositSweepTask/RedemptionTask. The confirmed correctness bug the cache
caused (candidates lost across calls) is fixed in this PR; dropping the
cache entirely is a larger architectural change with its own RPC-cost
tradeoff and is deferred to a follow-up rather than bundled into this
fix.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants